fix: scheduled task can run twice when an approval lands on a tick - #379
Open
Saidheerajgollu wants to merge 1 commit into
Open
fix: scheduled task can run twice when an approval lands on a tick#379Saidheerajgollu wants to merge 1 commit into
Saidheerajgollu wants to merge 1 commit into
Conversation
…awned run A tick's due() snapshot still lists a task whose run is parked on an approval — next_run only advances on completion. The overlap guard lived inside the spawned coroutine, so when an approval landed just before a tick, the parked run could finish and clear the guard before the duplicate spawn took its first step, and the task ran twice. This is the intermittent 'assert 2 == 1' in test_blocked_run_does_not_stall_other_tasks on main's CI. The new regression test forces that interleaving deterministically.
Author
|
Another hit today, on an unrelated branch in my fork running the same workflow: https://github.com/Saidheerajgollu/openworker/actions/runs/30665681535 — same |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Main's CI has been failing intermittently on
test_blocked_run_does_not_stall_other_taskswithassert 2 == 1— e.g. run 30587970128 (the #356 merge) and 30570317824. It's not a flaky test; the scheduler can actually run a task twice.The sequence:
next_runonly advances after completion.due()and spawns a duplicaterun_task. The overlap guard is checked inside the spawned coroutine, not at spawn time.The fix claims the guard synchronously in
_tickbefore spawning (no await between check and add) and skips the spawn when a run is already in flight.run_taskkeeps the same claim for direct callers, sotest_scheduler_skips_overlapping_runbehaves as before.The new test forces the exact interleaving deterministically (tick → park → approve → tick → drain). It fails on current main every time and passes with the fix. The existing test passes unchanged since the duplicate spawn no longer exists. Full suite: 1016 passed, 1 skipped.